ci: add Brev launchable shellcheck and provider compatibility matrix#829
ci: add Brev launchable shellcheck and provider compatibility matrix#829elookpotts-nvidia merged 5 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds Brev compatibility automation: GitHub Actions workflows and Bazel test infra, a Claude-driven Brev provider compatibility runner (prompt/spec, setup script, tests), a disk-fill task, shellcheck integration, README compatibility matrix updates, and CI guardrails and cleanup steps. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Repo as Repository
participant Brev as Brev CLI/Service
participant Claude as Claude Code
participant Instance as Remote Instance / K8s
GH->>Repo: checkout, read `deployments/brev/prompt.md`
GH->>Brev: install/login, install Claude skills
GH->>Claude: run rendered prompt via `npx `@anthropic-ai/claude-code``
Claude-->>GH: plan (instances, tests, README update)
GH->>Brev: create instances (parallel)
Brev-->>GH: instance IDs / endpoints
GH->>Brev: `brev exec` -> run `deployments/brev/setup.sh` on instances
Brev-->>Instance: deploy OSMO components
loop per-instance polling
GH->>Instance: poll pod/workflow status
Instance-->>GH: status updates
end
GH->>Repo: validate git diff (guardrail)
alt README changed
GH->>Repo: commit & push `deployments/brev/README.md`
end
GH->>Repo: verify `compat-result.txt` (PASS/FAIL)
GH->>Brev: delete created instances (cleanup)
Brev-->>GH: deletion results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
8234011 to
0791e78
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/brev.yml:
- Around line 69-72: The workflow step named "Install Brev CLI" currently pipes
a script from the mutable main branch into bash (curl | bash); replace that with
downloading the published GitHub Release tarball for the desired Brev version,
verify the release checksum/signature, extract the binary into $HOME/.brev/bin
(create the directory if needed), and then append $HOME/.brev/bin to
GITHUB_PATH; ensure the run block references a fixed release tag/version rather
than main and includes explicit checksum verification before extraction.
- Around line 111-116: The readiness check assigns NOT_READY from a pipeline
using grep -vcE which can exit non-zero when there are no matches, causing the
current "|| echo \"error\"" fallback to inject "error" and break the equality
check against "0". Change the pipeline that sets NOT_READY (the
ssh/kubectl/awk/grep pipeline) so it always returns a numeric count on success
or fallback to "0" on failure — for example replace the grep-count approach with
a robust alternative (e.g., pipe through wc -l or append "|| true"/"|| echo 0"
to the pipeline) so NOT_READY is reliably a number and the if [ "$NOT_READY" =
"0" ] check (and the readiness loop) works correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 435ac92f-5aed-4b15-a42e-8e405805a0b7
📒 Files selected for processing (2)
.github/workflows/brev.ymldeployments/brev/disk-fill-test.yaml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #829 +/- ##
=======================================
Coverage 42.82% 42.82%
=======================================
Files 203 203
Lines 27123 27123
Branches 7759 7759
=======================================
Hits 11616 11616
Misses 15398 15398
Partials 109 109 🚀 New features to boost your workflow:
|
|
📖 Docs preview: https://d3in15bfzp49i0.cloudfront.net/829/index.html |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deployments/brev/prompt.md`:
- Around line 54-56: The markdown has unlabeled fenced code blocks causing
markdownlint MD040; update each fence to include an appropriate language
identifier: change the block containing "brev exec <instance>
`@deployments/brev/setup.sh`" to use ```bash, change the block with "Workflow ID -
<id>" to use ```text, and change the table block beginning "| Provider |
Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes |" to use
```text (or ```markdown if you prefer table syntax highlighting); keep the
existing fence markers and only add the language tokens so the three snippets
are recognized by linters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dcc43dd7-6e43-43f2-97b0-e59dacc3020a
📒 Files selected for processing (8)
.github/workflows/brev.yml.github/workflows/pr-checks.yamlMODULE.bazeldeployments/brev/BUILDdeployments/brev/README.mddeployments/brev/prompt.mddeployments/brev/setup.shdeployments/brev/shellcheck_test.sh
✅ Files skipped from review due to trivial changes (1)
- deployments/brev/setup.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/brev.yml
a6904a6 to
38426c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
deployments/brev/prompt.md (1)
54-56:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks (MD040).
These three fenced blocks are unlabeled and will keep triggering markdownlint.
🛠️ Proposed fix
-``` +```bash brev exec <instance> `@deployments/brev/setup.sh`-
+text
Workflow ID --``` +```text | Provider | Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes |</details> Also applies to: 79-81, 121-123 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@deployments/brev/prompt.mdaround lines 54 - 56, The three unlabeled fenced
code blocks need language identifiers to satisfy markdownlint MD040: update the
block containing "brev exec@deployments/brev/setup.sh" to use
bash, the block with "Workflow ID - <id>" to usetext (ortext/plain), and the block with the table header starting "| Provider | Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes |" to usetext (orto locate the blocks and add the appropriate language tag to the opening fence (also apply the same change to the other occurrences noted around the same sections).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deployments/brev/prompt.md`:
- Around line 39-41: The example instance name uses an a100 GPU but the prompt
scope targets L40/L40S; update the example to match the selected GPU families by
replacing the a100 example with an L40-family slug (e.g., change
`osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-a100-2g` to something like
`osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-l40-2g` or `...-hyperstack-l40s-2g`)
so the example aligns with the naming pattern
`osmo-compat-{{GITHUB_RUN_ID}}-<provider>-<gpu-slug>`.
In `@deployments/brev/README.md`:
- Around line 59-63: The HTML comment block in the README contains a stray
double-quote just before the comment terminator (the line with Note: run all
brev commands...), which breaks the comment; open the block around the snippet
containing the cla ude/Note lines and remove the extra `"` so the comment ends
with a proper -->; verify the comment now reads as a well-formed HTML comment
and no other characters were altered.
---
Duplicate comments:
In `@deployments/brev/prompt.md`:
- Around line 54-56: The three unlabeled fenced code blocks need language
identifiers to satisfy markdownlint MD040: update the block containing "brev
exec <instance> `@deployments/brev/setup.sh`" to use ```bash, the block with
"Workflow ID - <id>" to use ```text (or ```text/plain), and the block with the
table header starting "| Provider | Instance Type | GPU | Hello World | Disk
Fill | GPU Workload | Notes |" to use ```text (or ```md/table) so all fenced
blocks are labeled; search for those exact snippets to locate the blocks and add
the appropriate language tag to the opening fence (also apply the same change to
the other occurrences noted around the same sections).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90be0525-bd1b-4bc6-b837-731c494e0dad
📒 Files selected for processing (9)
.github/workflows/brev.yml.github/workflows/pr-checks.yamlMODULE.bazeldeployments/brev/BUILDdeployments/brev/README.mddeployments/brev/disk-fill-test.yamldeployments/brev/prompt.mddeployments/brev/setup.shdeployments/brev/shellcheck_test.sh
✅ Files skipped from review due to trivial changes (6)
- deployments/brev/shellcheck_test.sh
- deployments/brev/disk-fill-test.yaml
- deployments/brev/setup.sh
- .github/workflows/pr-checks.yaml
- deployments/brev/BUILD
- .github/workflows/brev.yml
There was a problem hiding this comment.
♻️ Duplicate comments (4)
deployments/brev/prompt.md (4)
121-123:⚠️ Potential issue | 🟡 MinorAdd language identifier to fenced code block.
The code block lacks a language identifier, triggering markdownlint MD040.
🛠️ Proposed fix
-``` +```text | Provider | Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes |</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@deployments/brev/prompt.mdaround lines 121 - 123, The fenced code block
containing the table header line "| Provider | Instance Type | GPU | Hello World
| Disk Fill | GPU Workload | Notes |" is missing a language identifier; add a
language tag (e.g.,text ormarkdown) immediately after the opening
backticks so the block becomestext (ormarkdown) to satisfy markdownlint
MD040 and avoid the lint warning.</details> --- `54-56`: _⚠️ Potential issue_ | _🟡 Minor_ **Add language identifier to fenced code block.** The code block lacks a language identifier, triggering markdownlint MD040. <details> <summary>🛠️ Proposed fix</summary> ```diff -``` +```bash brev exec <instance> `@deployments/brev/setup.sh` ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@deployments/brev/prompt.mdaround lines 54 - 56, The fenced code block
containing the command "brev exec@deployments/brev/setup.sh" is
missing a language identifier; update that markdown block to use a bash language
tag (e.g., change the openingtobash) so the snippet is properly
recognized and MD040 is resolved.</details> --- `41-41`: _⚠️ Potential issue_ | _🟡 Minor_ **Update the instance-name example to match the GPU count.** The example shows `2g` (2 GPUs) but Phase 1 (lines 30-31) specifies "1 GPU each". The example should use `1g` to align with the spec. <details> <summary>🛠️ Proposed fix</summary> ```diff -(e.g. `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-a100-2g`) +(e.g. `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-l40-1g`) ``` </details> *Note: This also addresses the past review comment about using `a100` instead of L40/L40S.* <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@deployments/brev/prompt.md` at line 41, Update the instance-name example string in deployments/brev/prompt.md to match Phase 1's "1 GPU each" by changing the suffix from `-2g`/`2g` to `-1g` (i.e., use `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-l40-1g` → `...-hyperstack-<flavor>-1g`), and also apply the earlier hardware naming change by using `a100` instead of `l40`/`l40s` in the example so the final example reads like `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-a100-1g`. ``` </details> --- `79-81`: _⚠️ Potential issue_ | _🟡 Minor_ **Add language identifier to fenced code block.** The code block lacks a language identifier, triggering markdownlint MD040. <details> <summary>🛠️ Proposed fix</summary> ```diff -``` +```text Workflow ID - <id> ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@deployments/brev/prompt.mdaround lines 79 - 81, Update the fenced code
block that contains the literal "Workflow ID - " to include a language
identifier (e.g., changetotext) so markdownlint MD040 is satisfied;
locate the block containing the exact string "Workflow ID - " in prompt.md
and replace the opening fence with ```text.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In@deployments/brev/prompt.md:
- Around line 121-123: The fenced code block containing the table header line "|
Provider | Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes
|" is missing a language identifier; add a language tag (e.g., ```text or```text (or ```markdown) to satisfy markdownlint MD040 and avoid the lint warning. - Around line 54-56: The fenced code block containing the command "brev exec <instance> `@deployments/brev/setup.sh`" is missing a language identifier; update that markdown block to use a bash language tag (e.g., change the opening ``` to ```bash) so the snippet is properly recognized and MD040 is resolved. - Line 41: Update the instance-name example string in deployments/brev/prompt.md to match Phase 1's "1 GPU each" by changing the suffix from `-2g`/`2g` to `-1g` (i.e., use `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-l40-1g` → `...-hyperstack-<flavor>-1g`), and also apply the earlier hardware naming change by using `a100` instead of `l40`/`l40s` in the example so the final example reads like `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-a100-1g`. - Around line 79-81: Update the fenced code block that contains the literal "Workflow ID - <id>" to include a language identifier (e.g., change ``` to ```text) so markdownlint MD040 is satisfied; locate the block containing the exact string "Workflow ID - <id>" in prompt.md and replace the opening fence with ```text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID:
9f4688fd-0434-44f5-9664-dde5ba8e9aa0📒 Files selected for processing (1)
deployments/brev/prompt.md
Description
Adds CI for the Brev launchable (
deployments/brev/).Shellcheck runs on every PR touching
deployments/brev/**viabazel test //deployments/brev/...(hermetic shellcheck binary, required check inpr-checks.yml).Provider compatibility (
provider-compatjob) provisions L40/L40S instances across all Brev providers and runs hello world, disk fill (nvcr.io/nvidia/nemo:24.12, ~40 GB), and GPU workload tests, then commits results to the compatibility matrix indeployments/brev/README.md. The weekly cron is disabled until Brev supports long-lived API tokens.setup.sh: fixes Docker data-root detection to skip read-only filesystems (resolves Nebius failures where/mnt/cloud-metadatawas incorrectly selected).Issue #None
Checklist
Summary by CodeRabbit